px4io: When running HITL, don't publish actuator_outputs. Fixes #13471. #13488
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…3471. When running in HITL mode, pwm_out_sim publishes actuator_outputs. px4io unconditionally publishes the uOrb actuator_outputs. When HITL is configured, the px4io copy of the uOrb has all zeros. The result is that there are two publications, one valid, and one all-zeros. This causes the HIL_ACTUATOR_CONTROLS mavlink message to be incorrect (all-zeros) and the SERVO_OUTPUTS_RAW mavlink message to be inconsistent, as it takes the data from one or the other uOrb randomly each cycle. The fix is to let px4io know that HITL is in effect when it is started, and modify px4io to suppress publication in this case. This is a bigger more complicated fix than I would like, but I think it is conceptually correct. Signed-off-by: Nik Langrind <[email protected]>
|
Thanks, this is something that's been broken multiple times. |
LorenzMeier
approved these changes
Nov 17, 2019
|
@julianoes @dagar Do we want to pull this into 1.10? I'm tending towards it. |
bkueng
reviewed
Nov 18, 2019
| rc_handling_disabled = true; | ||
|
|
||
| } else if (!strcmp(argv[1], "hil")) { |
There was a problem hiding this comment.
This should be: strcmp(argv[extra_args], "hil"
There was a problem hiding this comment.
Oops, thanks, I will fix. Good eye!
Yes, I'll backport it. |
@bkueng could you please backport it. It's conflicting and I'm not aware of what has been going on here. |
|
@bkueng @julianoes - #13531 is a PR of a cherry pick from master into release/1.10 with the conflict resolved. I hope this makes your work easier. But if not, please decline it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Issue 13471 - vehicle doesn't take off in HITL SIM
The reason it fails:
When running in HITL mode, pwm_out_sim publishes actuator_outputs.
px4io unconditionally publishes the uOrb actuator_outputs. When HITL
is configured, the px4io copy of the uOrb has all zeros.
The result is that there are two publications, one valid, and one
all-zeros. This causes the HIL_ACTUATOR_CONTROLS mavlink message
to be incorrect (all-zeros) and the SERVO_OUTPUTS_RAW mavlink
message to be inconsistent, as it takes the data from one or the
other uOrb randomly each cycle.
The fix is to let px4io know that HITL is in effect when it is
started, and modify px4io to suppress publication in this case.
This is a bigger more complicated fix than I would like, but I
think it is conceptually correct.
An alternative would be to change src/modules/mavlink/mavlink_messages.cpp so that
MavlinkStreamHILActuatorControls() explicitly subscribed to instance 1 of the uOrb:
That's a one-line solution, but I think it is incorrect because nothing enforces which pub of the uOrb is instance 0 and which is instance 1. And, I just think it doesn't make sense to have two pubs.
Test data / coverage
I simply verified that the failure case in #13471 is fixed. I also verified that the Pixhawk still boots when HITL is not enabled.
Signed-off-by: Nik Langrind [email protected]